seems to build fine with the last commit (8b29310)

This looks promising.

Looks interesting to me too. Have you any ideas if it can also work with the mbedtls integration with the Microchip ATECC608 secure element which is presently only available with esp-idf?
https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/peripherals/secure_element.html

I've been trying this and it works well for me with TinyGSM. Would be great if it could be merged. @me-no-dev ?

yup :) in a bit. Wanted to merge S3 support first.

Unit Test Results

0 files  0 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌

Results for commit d75852b.

♻️ This comment has been updated with latest results.

There seem to be some issues with S3. I'll have a look. So this seems like a new implementation to replace WiFiClientSecure? Why not modify the original class instead? Having two classes that do the same thing (for the majority of users) will be confusing.

I think if WiFiClientSecure could be implemented as a thin wrapper over SSLClient, this would be a nice approach.

First, we would still have just one implementation (SSLClient).
Second, both the old code which uses WiFiClientSecure and the new code with SSLClient would continue to work.

However to achieve this we have to ensure that SSLClient behaves the same way as WiFiClientSecure, or that a backwards-compatible WiFiClientSecure wrapper would be possible to write.

Another point, I think this decision should be synchronized with esp8266/Arduino project. It wouldn't be great to introduce incompatibilities between the two cores intentionally.

By the way, there is also https://github.com/OPEnSLab-OSU/SSLClient library which works with many official Arduino boards. @sgalabov do you know of this library? Is choosing the same name intentional?

@igrr, @me-no-dev, my needs were to be able to use SSL on top of both WiFi and GSM (via TinyGSM), so when I had a closer look at WiFiClientSecure it was logical for me to try and rework it into something that could work with any Client based class and yes, the idea was to go on and convert WiFiClientSecure to work with SSLClient on top of WiFiClient, but it turned out I had to change HTTPClient as well and I just haven't been able to find the time to do that yet (and don't know when/whether I'll be able to, to be honest)... other than that I basically used WiFiClientSecure as a base, including its README and its examples :-)

@igrr, I didn't know about https://github.com/OPEnSLab-OSU/SSLClient so the name is more coincidental than anything else - just thought it fitted in well with its purpose :-)

In the meantime someone posted (although I can't find the message here) about https://github.com/govorox/SSLClient which seems to be more or less the same approach and idea as what I have submitted. I wish I had known about this before - maybe it would have saved me a bit of work...

I thought about the correct place to have such a lib and my opinion at the time was that it belongs with the Arduino support for ESP32 as this is the main target for the lib and would be a logical place to look for something like this for others :-)
Also, I am not sure I will have time to support something like this as open source and would much rather see it integrated in something bigger and with a well established community, which is why I have submitted it here.

There seem to be some issues with S3. I'll have a look. So this seems like a new implementation to replace WiFiClientSecure? Why not modify the original class instead? Having two classes that do the same thing (for the majority of users) will be confusing.

WiFiClient wificlient;
SSLClient  client(wificlient); <--- this would be our new WiFiClientSecure

Maybe WiFiClientSecure could be just a wrapper of SSLClient(WiFiClient). Just for backwards compatibility.

For this to be complete as @sgalabov said, HTTPClient should use Client instead of WiFiClient (like the standard ArduinoHttpClient library). That way you could use an SSLClient on top of a WiFiClient in the included HTTPClient from this repository.

SSLClient.zip

This would be the same @sgalabov uploaded but with the latest WiFiSecureClient changes incorporated (specifically, certificate bundle).

The only example that does not work is the one relying on HTTPClient because it was developed to work with WiFiClient and not with Client

It would probably be worth it to have a discussion on where and when this needs to go and then take the time to rework it based on the current WiFiClientSecure and rework everything else that needs to be touched, so that WiFiClientSecure becomes equivalent to SSLClient on top of WiFiClient and HTTPClient (at least) is taught about that. Probably SSLClient can be part of the WiFiClientSecure lib, although I personally tend to prefer them to be separate as it would be more visible and understandable what each one does...
What do you guys think?

Sounds good to me. I'm doing some more testing with SSLClient this week as it looks like there might be a subtle difference it the way it behaves compared to WiFiClientSecure which I'm trying to get to the bottom of.

@sgalabov I have fixed and updated your branch. Please pull the changes on your end.

@sgalabov I have fixed and updated your branch. Please pull the changes on your end.

not sure what you mean - I can already see your changes in my branch :-)

@sgalabov I have fixed and updated your branch. Please pull the changes on your end.

not sure what you mean - I can already see your changes in my branch :-)

I guess your git client already pulled them (mine does too)

I'm doing some more testing with SSLClient this week as it looks like there might be a subtle difference it the way it behaves compared to WiFiClientSecure which I'm trying to get to the bottom of.

What this looks like is that when an SSLClient.connect fails then SSLClient calls stop() which frees up all the resources and you can't use that SSLClient instance again and if you try it fails "Client not initialized" from line 91 in ssl_client.cpp, whereas with WifiClientSecure you can reuse it after a failed connection.

What this looks like is that when an SSLClient.connect fails then SSLClient calls stop() which frees up all the resources and you can't use that SSLClient instance again and if you try it fails "Client not initialized" from line 91 in ssl_client.cpp, whereas with WifiClientSecure you can reuse it after a failed connection.

Yes the behaviour is not the same. Because when you create an SSLClient, you pass the client for the connection. But when you stop() the SSLClient, it clears the passed client internally (in stop_ssl_socket() you will see ssl_client->client = nullptr; at the end). In WiFiClientSecure this worked because it didn't use clients, it used sockets and created the connections itself. But SSLClient needs a new client to work with if you stop it. Your best bet with this library as is, it to delete the SSLClient and recreate a new one.

Not sure if this is the best approach, but it is what it is haha. This SSLClient idea is great but it needs more work.

EDIT:
Something like this could be better so that SSLClient does not looses its client on stop().

// save only interesting field
int timeout = ssl_client->handshake_timeout;
Client *client = ssl_client->client;
// reset embedded pointers to zero
memset(ssl_client, 0, sizeof(sslclient_context));
// restore intesting fields
ssl_client->client = client;
ssl_client->handshake_timeout = timeout;

Another thing, SSLClient doesn't initialse _use_insecure, it should have bool _use_insecure = false; in SSLClient.h, otherwise the first requests i do get "WARNING: Skipping SSL Verification. INSECURE!".

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ lucasssvaz
❌ sgalabov
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

👋 Hello sgalabov, we appreciate your contribution to this project!

Generated by 🚫 dangerJS against f26f08d

@sgalabov Would it be possible to update this PR to 3.0.0 ?

@sgalabov will you be able to update this PR? Also please sign CLA. Thanks

Sorry, I won't be able to do that due to other commitments... I had forgotten about this PR and have no time to come back to it at the moment, so I am closing it.